fix(helm): use build path for charts for helm modules converted to ac… #5190
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@twelvemo The tests are failing. Can you please check those? |
Just did. The failing integ test about includes in helm charts was actually wrong. It was changed here to match the behavior resulting from the bug of comparing the node module path for helm charts and setting the includes according to it. Fixed the test. |
shumailxyz
reviewed
Oct 5, 2023
shumailxyz
reviewed
Oct 5, 2023
shumailxyz
approved these changes
Oct 5, 2023
shumailxyz
reviewed
Oct 5, 2023
| @@ -210,7 +210,7 @@ export async function configureHelmModule({ | |||
| ] | |||
| } | |||
|
|
|||
| const yamlPath = join(module.path, chartPath, helmChartYamlFilename) | |||
| const yamlPath = join(moduleConfig.path, chartPath, helmChartYamlFilename) | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…tions
What this PR does / why we need it:
Fixes a compatibility issue when using Helm modules. In Garden 0.12.x Helm modules always had an implicit build and charts and their dependenies were resolved in the build directory under
.garden/build/<module-name>. However Helm deploy actions and their dependencies are resolved in the action directory. When converting from modules to actions, we were creating adummyBuildwhich is a build of type exec for some scenarios e.g. when abasechart was explicitly set in the Helm module. When this was not the case the converted Helm deploy action would use the action directory for all Helm operations. This leads to a compatibility issue e.g. when referencing local dependency charts with relative paths. This PR creates adummyBuildfor every converted Helm module, so that every converted Helm module still uses the build directory.It also fixes a bug, where the
module.pathwas referencing the path of the node module and not the garden module, which ismoduleConfig.pathin the context of this function. This resulted in only a part of the Helm chart to be copied to the build directory.Adds tests to check if the correct base directory is used for both converted modules and native actions.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: